tools, build system: introduce PYTHON3_WITH_PEP723#22223
Conversation
maribu
added
Type: enhancement
|
Am I special? 🤣 After installing |
|
Perhaps it would be a good idea to add |
|
This is Ubuntu 22.04.5 LTS on WSL btw: |
|
You could run it with |
|
Maybe we should go for I think |
|
pipx version 1.0.0 is from January 2022, so a good 1.5 years older than the PEP 👀 I installed pipx fresh with |
I +1 a transition to uv, esp. since there is also a uv plugin for tox and I find our python tox stuff to be quite flaky since it doesn't have to benefits that uv brings to python envs like very exact version pinning. I also heard from @LasseRosenow that uv has become the quasi universal standard for modern python environments (Back in my days ppl used poetry 😔) |
|
How about adding a shell script in front that tries to identify which ever python venv management is installed and than runs the python script through that and as a last resort just calls the script on its own to check whether the user just does not want a venv manger but had installed the dependency trough their package manger (no one would use pip). -- instead of adding a new dependency on a specific python venv manger |
This allows users to not having to care about python dependencies. In addition SPDX annotations have been added.
- Document that Python scripts should use [PEP 723 Inline script metadata][pep723] - Add `PYTHON3_WITH_PEP723` as variable to allow uers to switch their running, such as `uv run` or `pipx run` or even just `python3` - Document that in the CI preparing a venv on demand is not acceptable, set `PYTHON3_WITH_PEP723` to `python3` in the CI [pep723]: https://peps.python.org/pep-0723/
maribu
force-pushed
the
dist/tools/usb-serial/pipx
branch
from
2ee9ec4 to
e3604b8
Compare
github-actions
Bot
added
Area: doc
maribu
changed the title
maribu
added
the
Process: API change
|
Would this mean, that we don't have to install the python dependencies for certain packages in the riotbuild docker container anymore? Which would also mean we don't have to set up the VENV stuff to update to a newer Python version? Which would also mean we can update the Ubuntu version? |
I think there we have a choice: Not install anything there any more, or install everything and set |
If that's holding us back, that "everything we'll ever need" could also be installed in a venv in the container that is active from the start. |
The issue that's holding us back is that nobody wants to tackle it, but @kaspar030 did something with I'd skip 24.04 LTS though and go to 26.04 LTS now. |
Co-authored-by: crasbe <crasbe@gmail.com>
kfessel
left a comment
kfessel
left a comment
There was a problem hiding this comment.
new dependancy intrdduced
| @@ -1,4 +1,15 @@ | |||
| #!/usr/bin/env python3 | |||
| #!/usr/bin/env -S uv run | |||
There was a problem hiding this comment.
isn't the entire point of PEP723 to not introduce a dependancy on a specific tool
There was a problem hiding this comment.
Yes But.
Python does not ship any out of the box that qualifies for the shebang.
All our invocations of any of our Python scripts happen through the PYTHON3_WITH_PEP723 executor and thus ignore the shebang.
The use that remains for the shebang (or, really, the executable bit set on the file) is for developers while they work on that script, or when they invoke it directly for some other reason. Nothing we set in there will work for everyone, but that path is not taken by RIOT itself, so just placing something in there is no new dependency, and anyone who invokes it and runs into trouble can just prefix it with the tool of their choosing.
Would it be better if python3 just did this? Yes. But that's not in there now, not sure if planned, and even if, some time down the road until users have it. Until then, we can make the best possible guess there, or leave it to the respective script's maintainers to pick any that works for them.
There was a problem hiding this comment.
Please read the cover letter of the PR first before commenting.
There was a problem hiding this comment.
⬆️ I was referring to Karl. @chrysn clearly did.
There was a problem hiding this comment.
but uv is just a random choice
and a user might have their package manager mange their python dependencies
why should the shebang point to uv and not just python which with the right dependencies installed is capable of executing a python-script
since our automatic exec happens through a PEP runner (selectable) anyway there is no reason to put that into the shebang
There was a problem hiding this comment.
The optimization goal for the she-bang is to pick something that is simple to maintain and works well for the majority of users.
Asking users to install uv is much easier than asking users to install a long list of python modules. Especially when the documentation containing the long list of python modules becomes out of date or contains modules that easily conflict with each other.
There was a problem hiding this comment.
why uv and not npm or pip or pipx or poetry or nix or ...
the python packagemanger should be a user choice a python script should not care
not all packemanger are able to read pep but if they install a python and the dependancies the *system python is able to exec that script
There was a problem hiding this comment.
In the build system we can control how scripts are executed. The proposal here is to just use PYTHON3_WITH_PEP723 as knob that allows users to pick what they please, while picking a sane default that is going to work for most users.
The shebang format however expects a single interpreter. So we have to make a choice here.
Here are the options for the list that I consider as serious suggestions, and what pros/cons I see for them as shebang:
- Use
python3directly:- Pros:
- Karl is happy
- Cons:
- Not one dependency to care about, but many
- Not running into a missing tool once, but for each and every script
- Documentation may get stale, needs to manually be kept in sync with the requirements in code
- Pros:
- Use
pipx run:- Pros:
- Only a recent version of
pipxneeds to be installed
- Only a recent version of
- Cons:
- Footgun: Ubuntu 24.04 ships a version of
pipxthat is not compatible with PEP 723 and fails with cryptic errors - Needs network access the first time you execute a script
- Relatively slow the first to you run a script
- Footgun: Ubuntu 24.04 ships a version of
- Pros:
- Use
uv run:- Pros:
- Only a recent version of
uvneeds to be installed - Very portable and easy to install (e.g. even on a raspberry pi without root permissions installing it was a one-liner)
- Orders of magnitude faster than pipx
- Only a recent version of
- Cons:
- Needs network access the first time you execute a script
- Pros:
There was a problem hiding this comment.
how is this
a valid way of installing something (at least they added a you might want to take a look first box and do not default to we want to be placed in /bin aka sudo needed)
Its is not even like clicking on every executable (something people get made fun of after security incidents) it is injecting a download directly into your system no rubber-ducky needed.
maybe we should add the download ahead of the shebang just to be sure.
There was a problem hiding this comment.
I'd like to add to suggestion 1
default to python3 directly and add documentation that one might want to run python scripts with their favourite python runner
uv run dist/tools/usb-serial/ttys.py
or
pipx run dist/tools/usb-serial/ttys.py
or
check the dependency documented in PEP.. form at the beginning of the python file
|
I'm not fully following anymore, is the discussion still about this PR or about what would need to happen for docker to be updated to fully make use of PEP723? |
| #!/usr/bin/env python3 | ||
| #!/usr/bin/env -S uv run |
There was a problem hiding this comment.
use python to exec python scripts
kfessel
left a comment
kfessel
left a comment
There was a problem hiding this comment.
Testing procedure
./dist/tools/usb-serial/ttys.pyshould work withpyudevinstalled regardless of yet another py packagemaneger
the docker shall not make use of PEP723 as written in the PR description it is not good to setup a venv with every ci invokation |
| # choose `pipx run` or just `python3` instead. In case of just `python3`, there | ||
| # will be no PEP 723 support and users will just have to install the right | ||
| # python modules beforehand. | ||
| PYTHON3_WITH_PEP723 ?= uv run |
There was a problem hiding this comment.
| PYTHON3_WITH_PEP723 ?= uv run | |
| # your python runner we conveniently chose a default that supports PEP 723 but any other might do | |
| PYTHON ?= uv run |
as python without PEP 723 is a valid choice ( as illustrated by this PR and the documentation above)
further more it reduces the inconvenience if one does not want to use uv
to
PYTHON=python make ...
6 participants
Contribution description
PYTHON3_WITH_PEP723as variable to allow uers to switch their running, such asuv runorpipx runor even justpython3PYTHON3_WITH_PEP723topython3in the CIttys.pyalready has been updated to follow the new styleTesting procedure
./dist/tools/usb-serial/ttys.pyshould work withoutpyudevinstalled, ifuvis installedmake -C examples/basic/default list-ttysshould work withoutpyudevinstalled, ifuvis installedPYTHON3_WITH_PEP723="pipx run" make -C examples/basic/default list-ttysshould work withoutpyudevinstalled, ifpipxis installed and somewhat recentPYTHON3_WITH_PEP723="python3" make -C examples/basic/default list-ttysshould work ifpyudevinstalled, regardless of whetherpipxoruvare installedIssues/PRs references
None
Declaration of AI-Tools / LLMs usage:
AI-Tools / LLMs that were used are: